Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add turret event #1237

Merged
merged 9 commits into from
Oct 12, 2019
Merged

Add turret event #1237

merged 9 commits into from
Oct 12, 2019

Conversation

neilzar
Copy link
Contributor

@neilzar neilzar commented Oct 2, 2019

When merged this pull request will:

  • Add a turret event that will send the event to the correct turret owner. For use with the commands that need to be run where the turret is local.

Still a bit unsure whether TUEVENT is a good choice, but I couldn't think of something else.

@commy2 commy2 added the Feature label Oct 2, 2019
@commy2 commy2 added this to the 3.13 milestone Oct 2, 2019
Co-Authored-By: commy2 <commy-2@gmx.de>
Copy link
Contributor

@commy2 commy2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works in local hosted MP.

@commy2
Copy link
Contributor

commy2 commented Oct 2, 2019

When does _turretOwner report 0?

@neilzar
Copy link
Contributor Author

neilzar commented Oct 2, 2019

I believe when there is no unit in the turret, haven't been able to properly test it in dedicated MP though. It is done this way in both ACE and ZEN, so I assumed they have found issues with it in the past.

I would assume that turretOwner would return the vehicle owner when the turret is empty because that is the turret locality at that time, but I guess there is some weird stuff in the engine that only returns the unit in the turret as the owner and throws 0 if there is no unit.

It only happens in SP, MP never returns 0 I believe.

Addendum: Driver and non-existent turrets also return 0. And since the driver is the vehicle owner, owner _vehicle returns the driver unit if there is one.

@commy2
Copy link
Contributor

commy2 commented Oct 3, 2019

In my testing it worked as expected, running on the owner of the vehicle when the turret was not crewed, despite this code block being bugged. I didn't test much with the driver turret [-1] though.

I suppose this would benefit from a MP unit test script.

@neilzar
Copy link
Contributor Author

neilzar commented Oct 10, 2019

Got to test this with one other player in local hosted MP, results were as expected. Tested it with a modified version of ZEN that uses the turret event to set the ammo on a vehicle. He placed a vehicle in Zeus, and I adjusted the ammo slider. These are the results I got:

  • When the vehicle was empty, it would run on his machine.
  • When a unit local to him was in any spot of the vehicle, it ran on his machine.
  • When a unit local to me was in any spot of the vehicle, it ran on my machine.
  • When his unit was driver and mine was in a turret, the events for the turret ran on my machine, and the one for the driver ran on his. Same the other way around.

These are the results that I expected.

@commy2
Copy link
Contributor

commy2 commented Oct 10, 2019

A change after I approved lies in how it handles non existing turrets I believe. What would the expected behavior be for non existing turrets? Before I approved the event would trigger nowhere. Now I think it falls back to vehicle locality, but I haven't tested.
Spontaneously I would say that a non existing turret should be treated like a null object in targetEvent and raise the event nowhere (silent fail).

@neilzar
Copy link
Contributor Author

neilzar commented Oct 10, 2019

I think a silent fail would be a good solution, I'll work on implementing it.

jonpas
jonpas previously requested changes Oct 10, 2019
addons/events/fnc_turretEvent.sqf Outdated Show resolved Hide resolved
neilzar and others added 2 commits October 11, 2019 01:31
Co-Authored-By: jonpas <jonpas33@gmail.com>
@commy2 commy2 dismissed jonpas’s stale review October 11, 2019 16:26

change request resolved

@commy2 commy2 merged commit e4a00b3 into CBATeam:master Oct 12, 2019
@neilzar neilzar deleted the turret-event branch October 12, 2019 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants